-
Notifications
You must be signed in to change notification settings - Fork 9.4k
[Backport] Fix disappearing navigation arrows in fotorama zoom #18595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Backport] Fix disappearing navigation arrows in fotorama zoom #18595
Conversation
Hi @luukschakenraad. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
I might be wrong about this, but it could be that this is trying to fix the same issue as #18443 is trying to fix (but that one is for a lot more options of fotorama) |
That one seems to be unrelated to this issue. This one has nothing to do with any settings. It's just a bug in Fotorama which causes wrong behaviour of the previous and next buttons. |
Well, actually they are related :) I just played around a bit with this, if I take a clean Magento 2.2.6 installation, then apply the changes from: And then re-test, all the issues you mention in #18585 are fixed. Here's why, these are the configuration values outputted in the html for the fotorama library: Vanilla 2.2.6: {
"fullscreen": {
"arrows": 1,
"loop": 1,
"nav": "thumbs",
"navdir": "horizontal",
"navtype": "slides",
"transition": "slide",
"transitionduration": 500
},
"options": {
"allowfullscreen": 1,
"arrows": 1,
"height": 560,
"keyboard": 1,
"loop": 1,
"nav": "thumbs",
"navarrows": 1,
"navdir": "horizontal",
"navtype": "slides",
"thumbheight": 110,
"thumbwidth": "88",
"transition": "slide",
"transitionduration": 500,
"width": "700"
}
} After applying two fixes mentioned above: {
"fullscreen": {
"arrows": true,
"loop": true,
"nav": "thumbs",
"navarrows": false,
"navdir": "horizontal",
"navtype": "slides",
"showCaption": false,
"transition": "slide",
"transitionduration": 500
},
"options": {
"allowfullscreen": true,
"arrows": true,
"height": 560,
"keyboard": true,
"loop": true,
"nav": "thumbs",
"navarrows": true,
"navdir": "horizontal",
"navtype": "slides",
"showCaption": false,
"thumbheight": 110,
"thumbwidth": 88,
"transition": "slide",
"transitionduration": 500,
"width": 700
}
} You'll notice that in the fixed code, a lot of values I'll leave this open for now, so a community maintainer can decide what to do with it, but in my opinion there is already a proper fix for the issue in #18443. |
You are absolutely right @hostep, very well spotted. It's my double not operator that transforms the number into a boolean and in #18443 the setting is directly set as boolean. In my opinion, both fixes can be applied as:
|
Ah yes, you are correct, I wasn't aware of this! Did it little more research, so just for some extra context:
Not sure why it got removed in Magento though ... But if we have to get it back in Magento, I suggest you also edit the |
I now changed the minified version as well. I did the same for pull request #18590 |
Hi @sidolov, thank you for the review. |
Hi @luukschakenraad. Thank you for your contribution. Please, consider to port this solution to 2.3 release line. |
Description (*)
Fix for dissapearing navigation arrows in Fotorama zoom
Fixed Issues (if relevant)
#18585: Navigation arrows zoomed fotorama disappear
Manual testing scenarios (*)
Contribution checklist (*)
Original PR
#18590